Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bugfix: Fixed conventional commit pre-commit hook not running. #395

Merged
merged 3 commits into from
Feb 7, 2022
Merged

Bugfix: Fixed conventional commit pre-commit hook not running. #395

merged 3 commits into from
Feb 7, 2022

Conversation

KaleabTessera
Copy link
Contributor

@KaleabTessera KaleabTessera commented Feb 7, 2022 •
edited
Loading

What?

Fix conventional commit pre-commit hook.

Why?

Invalid commit messages should be blocked at a commit level.

How?

Extra

Closes #394 .

@KaleabTessera KaleabTessera self-assigned this Feb 7, 2022
mmorris44
mmorris44 previously approved these changes Feb 7, 2022
View reviewed changes
arnupretorius
arnupretorius previously approved these changes Feb 7, 2022
View reviewed changes
Copy link
Collaborator

@arnupretorius arnupretorius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KaleabTessera 👍

@KaleabTessera KaleabTessera marked this pull request as draft February 7, 2022 08:18
AsadJeewa
AsadJeewa reviewed Feb 7, 2022
View reviewed changes
Copy link
Contributor

@AsadJeewa AsadJeewa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing on my side locally. I think we need to be explicit on the commit stage for each hook

.pre-commit-config.yaml Outdated Show resolved Hide resolved
@AsadJeewa AsadJeewa self-requested a review February 7, 2022 09:28
AsadJeewa
AsadJeewa suggested changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@AsadJeewa AsadJeewa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments for changes and also update (https://github.com/instadeepai/Mava/blob/develop/CONTRIBUTING.md)

.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
@AsadJeewa AsadJeewa dismissed stale reviews from arnupretorius and mmorris44 via 1a8f441 February 7, 2022 11:30
@AsadJeewa AsadJeewa marked this pull request as ready for review February 7, 2022 11:31
@AsadJeewa
Copy link
Contributor

I have updated with the changes that I requested above for review

@KaleabTessera
Copy link
Contributor Author

Thanks @AsadJeewa , the changes look correct 👍

arnupretorius
arnupretorius approved these changes Feb 7, 2022
View reviewed changes
Copy link
Collaborator

@arnupretorius arnupretorius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KaleabTessera and @AsadJeewa 🙂

@AsadJeewa AsadJeewa merged commit cb6b4f9 into develop Feb 7, 2022
@AsadJeewa AsadJeewa deleted the fix/conventional-commit-pre-commit-hook branch February 7, 2022 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AsadJeewa AsadJeewa AsadJeewa approved these changes

@arnupretorius arnupretorius arnupretorius approved these changes

@DriesSmit DriesSmit Awaiting requested review from DriesSmit

@jcformanek jcformanek Awaiting requested review from jcformanek

@RuanJohn RuanJohn Awaiting requested review from RuanJohn RuanJohn is a code owner

@mmorris44 mmorris44 Awaiting requested review from mmorris44

Assignees

@KaleabTessera KaleabTessera

Labels
size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Conventional Commit Pre-hook not working
4 participants
@KaleabTessera @AsadJeewa @arnupretorius @mmorris44